TEMPORARY: verify byok+license precedence (close me) - #107

…ence

Four things the Codex review was right about:

- The credential check is a Python program and ran before `setup-python`. A
  hosted runner ships a system python3 and would never notice; a self-hosted one
  without it would fail a valid configuration. Python is provisioned first now,
  which still leaves the check ahead of the engine install.
- `Post review failure` writes the same sticky comment on `failure()`, so a run
  stopped for a missing secret posted the fix and then buried it under "see the
  workflow logs". It now stands down when the credential check is what failed.
- The drift test treated any `agents.llm_config` import error as "engine not
  installed" and skipped, so a pin that moved a module would retire the very
  check meant to catch it. Installed-ness is asked of the distribution; an
  import failure after that is raised.
- The foreign-variable list spared every variable the selected provider could
  use rather than the ones the run resolved. `llm: openai` with only
  `openai_base_url` therefore let an inherited OPENAI_API_KEY credential the
  run: the same silent substitution this contract removes, one provider
  narrower.

Also switches this repo's own workflows from `llm: hosted` to `llm: license`.
CodeBoarding's repositories run on the CodeBoarding plan, and the secret is
already configured here. The README examples stay on `hosted`, which is the
right starting point for someone reading them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

docs/github-storage-scopes.md is an uncommitted local draft about Actions
storage scopes. It has nothing to do with the credential contract and was swept
in by a `git add -A`; it belongs wherever its author decides, not here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…eaches

Aliases were three extra values to document, test and keep in step with the
picker, in exchange for accepting a spelling nobody was asked to type. The table
is now keyed by the ONE value `llm:` accepts, each provider's inputs are named
after it (`llm: X` always pairs with `X_api_key`), and an unrecognised value is
refused with the accepted list.

Bedrock keeps the friendlier `aws_bedrock` rather than the engine's internal
`aws`; a `core` field carries that translation and nothing else does, so the
drift test still compares against the engine name exactly.

Also removed: the `hosted` field the resolver emitted and nothing read, and the
OIDC check in configure-auth.sh, which preflight had already made unreachable.
One decision point was the point.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`resolve_llm` described half the job. The module also writes every sentence the
user reads about a credential problem: preflight puts the message in the step
output, and action.yml posts that same string as the pull request comment, the
error annotation and the job summary. Naming it `llm_credentials` and saying so
in the docstring keeps the rule and its explanation together, which is the point
of having them in one file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Three more from the Codex review, all real:

- `"\n".join(...)` left foreign-envs unterminated, and `read` reports failure on
  an unterminated final line, so `while read` never ran its body for that
  record. VERCEL_BASE_URL sorts last, so an Anthropic run inherited it and core
  saw two providers configured. The list is terminated and the loop now handles
  a partial final line, either of which alone would fix it. The existing
  stripping test passed throughout because none of the variables it named was
  last, so the new one asserts the last entry specifically.
- Endpoint and region are first-class inputs now, but the reusable-analysis name
  hashed only provider and model, so pointing `openai_base_url` at another
  gateway could restore a warm start built against the old one. A backend id
  covering the tier, provider, endpoints and region feeds the state identity. It
  carries no key: rotating a secret must not throw away reusable analysis, and
  an artifact name must never be built from one.
- The README's input table still named `aws_region` after the rename.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`license_with_provider_key` could be raised and nothing asserted it, which is
what happens when a failure surface is only implicit in its raise sites. The
codes are declared in one frozenset now, ConfigError asserts membership, and a
test walks one configuration per code and asserts the set produced is exactly
the set declared, so adding a code without a case fails the build.

These codes are an interface: the action emits them as `llm_config_error` and
the webview keys on them, so adding one silently was a contract change nobody
reviewed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Agreed naming:

- supported-providers.json says what the list IS, which is the same set the
  refusal message recites back when a value is not recognised.
- credential_check.py leads with the verb, because refusing with a reason is the
  primary job. `resolve_llm` named only the resolving half, and the module also
  writes every sentence the user reads about a credential problem.
- verify-credentials.sh matches the verb-noun shape of its siblings
  (configure-auth, fetch-state, deliver-sync, render-review) rather than being
  the one noun-first script in the directory.

configure-auth.sh keeps its name for now, though after this change it only
starts the hosted relay.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

"Add the secret and wire it as X" describes the fix rather than being it. The
reader is often someone who has never edited a workflow, so a refusal now names
the exact secrets page as a link, the exact workflow file, and the YAML to paste,
indented as it will sit in the `with:` block.

That needs two renderings, because the surfaces differ: `::error::` annotations
cannot carry newlines, so `message` stays one plain line for the annotation while
`details` carries the markdown for the pull request comment and the job summary.

Only the refusals with one exact answer get a snippet. "Remove one of these" has
two valid fixes, so picking one would be guessing, and it stays prose.

The repository and workflow file come from GITHUB_REPOSITORY and
GITHUB_WORKFLOW_REF, so off a runner the remedy degrades to prose rather than
emitting a half-built link.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…heck crashes

Three things, all from questions the summary could not answer:

- `byok+license` runs on YOUR key, always. A direct provider call never reaches
  CodeBoarding, so there is nothing for a licence to pay for there; it is
  recorded and not spent. That was already the behaviour and nothing pinned it,
  so a test now asserts the provider key is exported, no licence is staged, and
  the relay is never started.
- "Tier: byok+license" names the configuration without answering the question a
  summary is read to answer. The summary now states which credential pays, says
  outright that a wired licence is not spent on a direct call, and names a
  non-default endpoint or region, which is the setting most likely to be wrong
  and least likely to be noticed.
- The credential check is `continue-on-error` so a refusal can be reported before
  the job dies. A crash inside it -- an unreadable table, a missing python3 --
  therefore left `error` empty and let the run reach the checkout and the engine
  install. The stop step watches the step outcome as well, so "credentials are
  decided before anything expensive happens" holds when the deciding breaks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>